feat(visual): base/head render harness reusing ui:preview - #759
feat(visual): base/head render harness reusing ui:preview#759Helios531 wants to merge 2 commits into
Conversation
|
gittensory · advisory review Reviewed 7 changed file(s) — two independent AI reviewers. Changed files: Suggested action: 🛠️ Request changes. (reviewers split: request changes / merge) Address the suggestions below before merging. Reviewer A · Suggestions
Worth double-checking
Reviewer B · Suggestions
Worth double-checking
|
JSONbored
added
the
gittensor:feature
|
Note Gittensory Gate skippedPR closed before full evaluation. No late first comment was created.
💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers. |
ghost
added
the
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #759 +/- ##
==========================================
+ Coverage 96.59% 96.61% +0.01%
==========================================
Files 91 93 +2
Lines 13646 13695 +49
Branches 4975 4993 +18
==========================================
+ Hits 13182 13231 +49
Misses 97 97
Partials 367 367 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![superagent-security[bot]](https://avatars.githubusercontent.com/in/3287076?s=60&v=4){kind=small}
superagent-security
Bot
added
the
pr:flagged
d0e2bae to
1c5300b
Compare
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
superagent-security
Bot
removed
the
pr:flagged
There was a problem hiding this comment.
PR #759 — feat(visual): base/head render harness reusing ui:preview
Verdict: Close
Resolves issue #580: NO
CI: Checks are green but they validate the WRONG code — green CI is misleading here.
Highlights:
- None applicable — the branch does not contain the described render-harness implementation.
Concerns / required changes:
- WRONG BRANCH CONTENT: the head commit (136ad23 "fix: gittensory-consume-metagraphed") adds src/services/metagraphed.ts + src/signals/subnet-claim.ts — i.e. this is PR #762's metagraphed work, duplicated. The only other commit is an unrelated main-merge.
- The files the PR body promises (src/visual/render-harness.ts, scripts/visual-render.ts) do NOT exist on the branch — confirmed via get_files, get_diff, and per-commit file lists.
- package.json adds a visual:render script pointing at scripts/visual-render.ts that isn't present, so visual:render would fail.
- Implements none of #580 (no two-ref ephemeral wrangler serve, no health-wait, no BASE_URL/HEAD_URL parameterization), and re-lands the #697 feature already covered by #762.
Completeness vs issue: Does not satisfy #580 at all. The branch was pushed/rebased onto the wrong work and must be rebuilt with the intended render-harness commits before it can be reviewed on its merits.
|
@JSONbored I have mixed with other issue. I would like to contribute here, Could you reopen this PR? |
2 participants
Summary
wrangler devservers (the same local-serve mechanismui:previewuses), health-checks each, and exposes their origins asBASE_URL/HEAD_URL— the inputs the owner-led(visual-agent): Playwright before/after capture across viewports #581 capture step consumes.src/visual/render-harness.ts+ thin runnerscripts/visual-render.ts+visual:rendernpm script.Related Issue
Closes: #580
Change Type (select all that apply)
Real behavior proof
The actual e2e log: both servers ready at HTTP 200, BASE_URL/HEAD_URL printed, probe hitting /, /roadmap, /changelog → 200 on both, exit 0, clean teardown. Plus the two issues found-and-fixed (.output vs dist path, process-group teardown).
Validation checklist
Honestly marked:
✅ git diff --check, typecheck, test:coverage (≥97%, new file 100%), ui:build (via real run).
⬜ others marked not run with a one-line reason each (tooling-only PR, unrelated subsystems — confirm in CI).